Skip to content

feat: add hardware-validated YCBT R10M support - #31

Merged
foureight84 merged 39 commits into
foureight84:mainfrom
DBozhinovski:test/ycbt-sleep-steps-20260717
Jul 25, 2026
Merged

feat: add hardware-validated YCBT R10M support#31
foureight84 merged 39 commits into
foureight84:mainfrom
DBozhinovski:test/ycbt-sleep-steps-20260717

Conversation

@DBozhinovski

Copy link
Copy Markdown
Contributor

Summary

  • add the YCBT / SmartHealth protocol stack and LittleMeatball R10M pairing support
  • implement capability-gated startup, history sync, live status, spot HR/SpO2/BP measurements, battery, activity, sleep, and REM
  • harden Android GATT subscription, serialization, teardown, timeout, and reconnect behavior without changing existing ring-family contracts
  • make history persistence reconnect-safe, preserve revised sleep sessions, and add a Room v7 to v8 duplicate-measurement cleanup
  • keep measurement settings and pairing claims aligned with firmware-declared capabilities

Hardware validation

Validated on R10M FCF4, firmware 2.32:

  • pairing, handshake, reconnect, and day rollover
  • activity and history sync
  • HR, SpO2, and BP history and spot measurements
  • battery, sleep stages, and REM rendering

The implementation intentionally does not advertise or query capabilities this firmware does not declare, including temperature, glucose, HRV, stress, fatigue, Find Device, and dedicated SpO2 history.

Verification

  • TZ=UTC ./gradlew --no-daemon testDebugUnitTest --rerun-tasks
  • TZ=Europe/Skopje ./gradlew --no-daemon testDebugUnitTest
  • ./gradlew --no-daemon :app:assembleDebug
  • manual SQLite validation of the v7 to v8 deduplication and unique-index statements
  • hardware testing on R10M firmware 2.32

Debug APK: app/build/outputs/apk/debug/app-universal-debug.apk

@foureight84

Copy link
Copy Markdown
Owner

We just had a massive update. Could you please resolve the conflicts?

@DBozhinovski

Copy link
Copy Markdown
Contributor Author

Absolutely, on it.

@foureight84

Copy link
Copy Markdown
Owner

Review notes

A few things I'd want addressed or answered before merge, grouped by severity.

High

1. handleTerminal count-mismatch silently stalls the transfer instead of retryingYCBTHistoryTransfer.kt

if (packets != expectedPackets || bytes != expectedBytes || bytes != buffer.size) {
    return emptyList()
}

On a metadata/byte-count mismatch this returns with no block ACK and no advance(). Since the ring won't release the next type until it receives 05 80 {status}, the transfer now hangs until the 10–30 s watchdog fires and skips the whole type. The previous code fell through to the CRC path, which sent ACK_CRC_FAILURE and got exactly one retransmit — so a single dropped data frame used to cost one retry and now costs a stalled type plus a lost history category. Suggest: on a count mismatch, send ACK_CRC_FAILURE and go through retryOrSkip(type) rather than returning empty.

Medium

2. CONNECTED no longer clears real sleep for any familyEventPersistenceSubscriber.kt + DataRepairs.kt

The CONNECTED handler now calls clearDemo() on sleep sessions/blocks instead of clear(). The rationale (YCBT re-emits CONNECTED repeatedly + async history ⇒ don't flash-then-vanish a night) is sound for YCBT, but this changes behavior for the other ring families, which previously wiped-and-rebuilt sleep on every connect — and the retired DataRepairs comment shows that wipe was load-bearing for cleaning up midnight-split / mis-keyed legacy sessions. This is validated on R10M only. Please confirm the other families don't accumulate stale/duplicate sleep sessions now, or gate the "preserve" behavior to RingDeviceType.YCBT.

3. MIGRATION_12_13 does a silent destructive dedup + unique indexPulseLoopDatabase.kt

DELETE FROM measurements WHERE rowid NOT IN (SELECT MAX(rowid) ... GROUP BY kindRaw, timestamp, sourceRaw)

then CREATE UNIQUE INDEX ... (kindRaw, timestamp, sourceRaw). timestamp is epoch millis, so this is usually safe, but it permanently drops any pre-existing rows that legitimately share (kind, ms, source). Worth confirming no existing family ever wrote two valid rows at the same kind+ms+source (combined-packet bursts, live sampling). Related: this PR relabels live HRV/temperature sourceRaw from "colmi""live", so already-persisted "colmi" rows and new "live" rows for the same kind+ms will both survive the unique index (parallel lineages). Probably cosmetic, but worth calling out.

4. forget() / factoryReset() dropped the stop() callRingSyncCoordinator.kt

Both paths went from client.forget(); stop() to just client.forget(). stop() cancelled streamJob/startupJob. Leaving them running means the coordinator keeps observing (and a startupJob may still be mid-flight) against a device being torn down. Intentional (to survive a re-pair)? If so a one-line comment helps; otherwise it's a late-write/leak risk.

5. RESPIRATORY_RATE / VO2MAX are decoded then unconditionally filtered outYCBTDriver.isSupported vs MetricsService.kt

combinedVitals/bodyData decode RESPIRATORY_RATE and VO2MAX, but YCBTDriver.isSupported maps both to false, so events.filter(::isSupported) drops them and nothing is ever persisted. Meanwhile MetricsService.isSupported was just changed to gate them on db.measurementDao().latest(kind) != null — which can now never be non-null for YCBT. So it's dead decode feeding an unreachable UI branch. Pick one: stop decoding them, or let them through.

Low / polish

6. RingBLEClient.onCharacteristicChanged now holds forgetLock across the entire driver.ingest() (full history-buffer CRC + decode) and every publishBlocking. The GATT callback thread is already serial, so the lock really only guards against finalizeForget() on another thread — consider narrowing its scope so it isn't held through a large history decode.

7. PulseEventBus.publishBlocking now does check(pending.trySend(event).isSuccess). The channel is UNLIMITED and never closed, so this is practically unreachable — but a check() throwing IllegalStateException from a BLE callback thread is a sharp edge; prefer logging over throwing there.

8. measureBloodPressure() hand-rolls the poll loop (for step in 0 until BP_MEASURE_SECONDS*2 { delay(500) }) while measureHR/measureSpO2/measureHRV all use pollForValue(...). Reuse pollForValue for consistency.

9. SPOT_MEASURE_SECONDS = HR + SpO2 + BP(40) + HRV(40) + 3 ≈ up to ~110 s, and measureSpot() runs the four legs sequentially with the optical sensor pinned. Early-exit mitigates the typical case, but the worst-case countdown is ~2 min — confirm that's the intended UX for rings that grant all four manual caps.

10. YCBTSyncEngine.writer was changed valvar but is never reassigned — revert to val.

11. decodeDeviceInfo now emits FirmwareVersion(major*100 + minor) (e.g. "1.05" → 105); the prior code deliberately avoided this int encoding as misleading. Fine if consumers treat it as opaque — just confirming.

12. In YCBTDriver.ingest, a MEASUREMENT_STATUS push carrying a real value clears the entire pendingMeasurementReplies FIFO. During a workout HR stream plus a concurrent spot measurement, that could drop an unrelated pending correlation. Edge case, but noting it.

@foureight84

Copy link
Copy Markdown
Owner

Protocol / wire-layer pass

Went through the framing, CRC16, opcode tables, history record offsets, and the support-function bitmap separately — the wire layer looks correct, so nothing blocking there. Two small things worth a look:

Unverified/possibly-wrong mode constantsYCBTMeasurementMode.BLOOD_KETONE = 0x07 and RESPIRATORY_RATE = 0x03 don't line up with the mode numbering the ring actually uses for those measurements, and BLOOD_KETONE in particular looks off. Both are inert today (nothing ever starts a ketone/respiratory-rate spot measurement), so there's no runtime impact — but worth double-checking before anything is ever wired to them.

Inferred conversions with no hard ground truth — the blood-sugar tenths-of-mmol → mg/dL conversion (bloodSugarMgdl, used by the 0x2f, 0x09, and 04/13 paths) and the stress/fatigue ×10 digit-concatenation in score() are interpretations rather than confirmed decodes; the code already carries UNVERIFIED notes on them. Just flagging that these are the decodes most likely to need a real hardware check, since a wrong scale would still land plausible-looking values.

Minor: the 06/00 live-status comment still labels distance/calories "UNVERIFIED (capture-inferred)" — those offsets are actually correct, so the comment is stale.

@DBozhinovski

Copy link
Copy Markdown
Contributor Author

Addressed all review notes in 849131d and merged the latest main in b0b5fd9.

  1. History terminal count mismatches now send ACK_CRC_FAILURE, retry once, then skip/advance. Added coverage for the mismatch/retry path.
  2. Sleep preservation is limited to the YCBT protocol stack. YCBT, TK5, and COLMI_SMART_HEALTH all use YCBTDriver and share its repeated status packets and asynchronous history transfer; packet-based Colmi/Jring/CRP families still clear and rebuild sleep on connect.
  3. Removed destructive measurement deduplication and the unique identity index. The migration now preserves legitimate same-millisecond collisions, normalizes the old HRV/temperature source lineage, and adopts deterministic primary keys only for replayed history rows.
  4. Forget/factory-reset now stop coordinator jobs before unbind, await ACK-or-timeout teardown, then restart the process-long event collector so same-process re-pairing still works.
  5. Respiratory-rate and VO2max history now pass capability filtering, plausibility validation, and persistence.
  6. forgetLock now protects only short generation/state checks. Decode, event publication, sync-engine handling, GATT teardown, preferences, and state updates run outside it.
  7. Event-bus send rejection logs instead of throwing from a BLE callback.
  8. Blood-pressure measurement now uses the shared polling helper.
  9. Confirmed: 173 seconds is the intentional worst-case UI countdown for sequential HR + SpO2 + BP + HRV. Capability gating and stable-reading exits shorten normal runs.
  10. Restored YCBTSyncEngine.writer to val.
  11. Removed the misleading integer firmware event from the YCBT device-info path; firmware remains the protocol-formatted string.
  12. Measurement pushes now remove only the pending correlation for the matching mode.

Wire-layer follow-ups:

  • Removed the inert, unverified blood-ketone and respiratory-rate spot-mode constants.
  • Restored explicit UNVERIFIED notes on blood-sugar scaling and stress/fatigue score composition.
  • Removed the stale distance/calorie uncertainty comment.

Additional lifecycle hardening found during device testing:

  • Intentional background disconnects no longer get undone by the 15-second watchdog.
  • Foreground and short-lived background clients share a process-wide GATT ownership gate, and workers yield promptly when the app foregrounds.
  • Worker-owned clients always destroy their watchdog/GATT resources.
  • Graceful teardown persists DISCONNECTED rather than leaving stale state.

Verification:

  • testDebugUnitTest passed.
  • :app:assembleDebug passed after merging current main.
  • git diff --check passed.
  • In-place Pixel 7 testing preserved app data and validated connect, history sync, foreground-worker exclusion, background teardown without watchdog reconnect, and foreground reconnect.
  • Migrated database is v14 with PRAGMA integrity_check = ok; ring identity, measurements, and sleep sessions were retained.

…r shadowing

Address the confirmed issues from the PR foureight84#31 review:

- YCBTHistoryTransfer: publish watchdog-driven events OUTSIDE the transfer
  monitor. Holding it while `onOutOfBandEvents` re-enters the sync engine
  (which calls back into `append`) inverted the engine->transfer lock order
  the GATT-callback path uses — a classic AB/BA deadlock that could wedge all
  BLE notification processing during history sync.
- YCBTCoordinator: narrow name matching to R10M only. It sits ahead of
  TK5Coordinator/ColmiSmartHealthCoordinator in the registry, so matching the
  TK5/T50/SR0x/R0x prefixes by name shadowed uncataloged units of those
  families and mis-bound them to RingDeviceType.YCBT. The be940000 service
  stays a positive signal (R10M-exclusive; not advertised by TK5/SmartHealth).
- PulseEventBus: isolate each emit so one throwing event can't kill the
  single, non-restarting dispatcher and silently stop every subscriber.
- RingBLEClient: read `inFlightOp` under `opLock` in the op-timeout path so a
  completion landing at the timeout can't trigger a spurious reconnect.
- YCBTDriver: mark the cross-thread `capabilities` field @volatile.
- RingDecodedEvent: correct the stale MeasurementRejected doc (it now maps to
  PulseEvent.MeasurementRejected via RingEventBridge).

Add regression tests locking in the R10M-only coordinator scope.

Left as-is by design: onCharacteristicWrite's immediate recoverWedgedLink on a
write failure (it can't tell a failed required-handshake write from a normal
one, and advancing risks retiring a successor on the shared callback channel).
# Conflicts:
#	app/src/main/java/com/pulseloop/ring/RingEventBridge.kt
#	app/src/main/java/com/pulseloop/service/EventPersistenceSubscriber.kt
#	app/src/main/java/com/pulseloop/service/RingSyncCoordinator.kt
#	app/src/main/java/com/pulseloop/ui/screens/DebugScreen.kt
Colmi HRV *history* used to persist as an `HrvSample` — random id, sourceRaw
'live' — and now decodes to a `HistoryMeasurement`, keyed on
`history:hrv:<timestamp>`. `adoptStableMeasurementIdentities` re-keyed the
equivalent TEMPERATURE rows but not HRV, so the old rows never collided with
the new ones: a re-sync wrote a second row at every timestamp already stored,
and `range()` filters on kindRaw + timestamp (never sourceRaw), so both came
back and the HRV series doubled.

Adds the missing 'live' pass for HRV, mirroring TEMPERATURE, plus a v14 → v15
migration so a test APK already on v14 re-runs the (idempotent) adoption
instead of keeping its un-keyed rows.

Also fixes the R10M catalog entry: it no longer borrows Colmi R10 product art
(it is a YCBT-protocol ring, not a Colmi one — it now falls back to the generic
silhouette like TK5/TK18), and leads with the model number the ring actually
advertises, "R10M (LittleMeatball)", since that is what every reseller of this
white-label ODM shares while the brand is what a buyer recognises.
…lated

Before measurements had stable ids, a ring's history replay was persisted with
a fresh random id on every sync, so each re-sync appended another row at a slot
already stored. On a real Colmi R10 that meant HRV, stress and temperature
growing by one row per slot per sync, permanently — nothing prunes this table.
A device pulled for this change held 1197 measurement rows covering only 414
distinct slots: 783 rows, 65% of the table, were replay copies.

They are not merely wasted space. `dailyAggregates`/`hourlyAggregates` compute
AVG(value) over raw rows with no sourceRaw filter, so a slot the ring replayed
more often than its neighbours drags the daily average toward its value.

`adoptStableMeasurementIdentities` already stops the growth by giving one row
per slot the canonical `history:<key>:<timestamp>` id that later syncs upsert
onto, but it deliberately leaves the accumulated copies behind. This removes
them, restricted to rows that are provably redundant: a non-canonical row goes
only when a canonical row exists for the same (kindRaw, timestamp) AND holds
the same value. A differing value is a distinct reading and is always kept, so
no information can be destroyed.

Interruption safety — one statement, so SQLite's journal makes it all-or-
nothing; a kill can never leave the table half-deleted. Room additionally runs
migrations inside SQLiteOpenHelper's onUpgrade transaction, so the version bump
and the delete commit together and an interrupted upgrade rolls back to v15 and
re-runs next launch. The statement is idempotent, so that retry is safe.

Verified against the real device database: 1197 -> 414 rows with every distinct
(kind, timestamp, value) triple preserved, a second run a no-op, a rolled-back
run leaving all 1197 intact and its retry completing cleanly, and PRAGMA
integrity_check ok. Then confirmed on-device: every kind now holds exactly one
row per slot.
@foureight84
foureight84 merged commit a98d595 into foureight84:main Jul 25, 2026
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants